-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Document extra reason to allow not existing dir in EXTRA_CONF_DIRS
, fix unable to change Twitcher logging level
#410
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we change up the wording a bit?
Maybe I'm just tired but it took me a while to understand what you meant initially.
CHANGES.md
Outdated
@@ -15,7 +15,12 @@ | |||
[Unreleased](https://github.com/bird-house/birdhouse-deploy/tree/master) (latest) | |||
------------------------------------------------------------------------------------------------------------------ | |||
|
|||
[//]: # (list changes here, using '-' for each new entry, remove this when items are added) | |||
## Changes | |||
- Code documentation: extra reason to allow not existing dir in `EXTRA_CONF_DIRS` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we change this to:
- Code documentation: provide an additional reason to not exit early if a directory listed in the
EXTRA_CONF_DIRS
variable does not exist.
(or similar)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, updated. English is not my native language!
birdhouse/read-configs.include.sh
Outdated
@@ -126,6 +126,14 @@ source_conf_files() { | |||
# fix immediately. | |||
# The new adir with typo will not be active but at least all the existing | |||
# will still work. | |||
# | |||
# Do not exit on not existing conf dir also allow for smooth |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested:
# Not exiting also allows for a smooth transition when component paths change between updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one I decided to keep the specific part about not existing conf dir
because it is code comment so should be more precise. New wording: "Allowing not existing conf dir also helps for smooth transition of component path when they are new/renamed/deleted."
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2359/Result : failure BIRDHOUSE_DEPLOY_BRANCH : update-code-comment DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-20.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1477/NOTEBOOK TEST RESULTS |
E2E Test ResultsDACCS-iac Pipeline ResultsBuild URL : http://daccs-jenkins.crim.ca:80/job/DACCS-iac-birdhouse/2360/Result : success BIRDHOUSE_DEPLOY_BRANCH : update-code-comment DACCS_CONFIGS_BRANCH : master PAVICS_E2E_WORKFLOW_TESTS_BRANCH : master PAVICS_SDI_BRANCH : master DESTROY_INFRA_ON_EXIT : true PAVICS_HOST : https://host-140-20.rdext.crim.ca PAVICS-e2e-workflow-tests Pipeline ResultsTests URL : http://daccs-jenkins.crim.ca:80/job/PAVICS-e2e-workflow-tests/job/master/1478/NOTEBOOK TEST RESULTS |
Changes
Non-breaking changes
EXTRA_CONF_DIRS
variable does not exist.birdhouse_daccs_configs_branch: master
birdhouse_skip_ci: false